Vtadmin2 - #20946
Open
thealish wants to merge 29 commits into
Open
Conversation
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Alisher <thealishh@gmail.com>
Signed-off-by: Alisher <thealishh@gmail.com>
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Contributor
Promptless documentation updates
|
Tighten VDiff/reload-schema authorization, treat unauthorized failover as a denial, and ship the remaining UI correctness and packaging fixes from the vtadmin2 review. Signed-off-by: Alisher <thealishh@gmail.com>
Signed-off-by: Alisher <thealishh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Alisher <thealishh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Alisher <thealishh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Alisher <thealishh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Alisher <thealishh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Multi-cluster forms retain keyspaces from the previously selected cluster, preventing several operations from targeting another cluster correctly.
Review details
Suppressed comments (6)
Previously missed (6) — in code that hasn't changed since the last review.
go/vt/vtadmin/vtadmin2/templates/migrations.html:28
- Changing the cluster leaves this select populated with keyspaces from the previously rendered cluster. Because no client-side code updates dependent selects, the request pairs the new cluster with the old cluster's keyspace, making migrations on non-default clusters inaccessible through the form. Please make cluster selection reload the keyspaces first or update the options dynamically.
go/vt/vtadmin/vtadmin2/templates/transactions.html:25 - Changing the cluster does not refresh this keyspace select: it contains only keyspaces from the cluster selected when the page was rendered, and the shipped JavaScript has no dependent-select handler. In a multi-cluster deployment, submitting a different cluster therefore sends a keyspace from the previous cluster, so users cannot select/query the new cluster through this form. Please add a two-step cluster selection or update the keyspace options when the cluster changes.
go/vt/vtadmin/vtadmin2/templates/vexplain.html:24 - This dropdown is rendered only for the current cluster, but changing the cluster dropdown does not repopulate it. Submitting VExplain for another cluster therefore sends the old cluster's keyspace and cannot target the selected cluster. Please implement a cluster-selection reload step or a dependent keyspace update.
go/vt/vtadmin/vtadmin2/templates/vtexplain.html:24 - This dropdown is rendered only for the current cluster, but changing the cluster dropdown does not repopulate it. Submitting VTExplain for another cluster therefore sends the old cluster's keyspace and cannot target the selected cluster. Please implement a cluster-selection reload step or a dependent keyspace update.
go/vt/vtadmin/vtadmin2/templates/workflow_materialize_create.html:30 - The source-keyspace options are restricted to the cluster selected at render time, but selecting another cluster does not update them. “Load reference tables” then submits the new cluster with a source keyspace from the old cluster, so Materialize creation cannot target a non-default cluster through the UI. Please reload the form on cluster selection or update this dependent select.
go/vt/vtadmin/vtadmin2/templates/workflow_movetables_create.html:30 - The source-keyspace options are restricted to the cluster selected at render time, but selecting another cluster does not update them. “Load tables” then submits the new cluster with a source keyspace from the old cluster, so a MoveTables workflow cannot be created for a non-default cluster through the UI. Please reload the form on cluster selection or update this dependent select.
- Files reviewed: 110/111 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Signed-off-by: Alisher <thealishh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Alisher <thealishh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The broad administrative surface includes numerous destructive cluster operations and warrants final human validation.
Review details
- Files reviewed: 112/113 changed files
- Comments generated: 0 new
- Review effort level: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related Issue(s)
#20107
Checklist
Deployment Notes
AI Disclosure
I have used GLM 5.3-flash extensively for UI work and for tests